fix(sandbox): distinguish descriptor lookup outcomes by johntmyers · Pull Request #4494 · NVIDIA/OpenShell

johntmyers · 2026-10-13T17:22:13Z

Summary

Distinguish verified unregistered descriptors from descriptor inspection failures in the sandbox network broker. Deny operations when the broker cannot verify the descriptor before a native fallback.

Related Issue

No issue, per direct request.

Changes

Preserve distinct procfs lookup outcomes for sockets, non-sockets, and inspection errors.
Apply fail-closed handling to connect, bind, listen, and send fallback paths.
Add regression coverage for an unreadable workload and native Unix socket behavior.

Testing

Linux sandbox compile and Clippy checks pass
Non-root Linux regression tests pass
Docker end-to-end run passes, including transparent_tcp
Repository commit hooks pass

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
Architecture docs updated (not applicable)

github-actions · 2026-10-13T17:36:53Z

Label test:e2e applied for 9231102. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

johntmyers · 2026-10-13T17:38:50Z

gator-agent

PR Review Status

The independent review found no blocking problems in the descriptor-inspection change. Lookup errors now deny native fallback, while verified unregistered descriptors retain native Unix socket behavior.

Blocking findings: None.
Carried findings: None.
Non-blocking suggestions: None.

Gator metadata

Validation: Focused sandbox network enforcement fix from a verified maintainer.
Docs: No published interface or configuration change; existing support documentation requires broker task visibility and fail-closed qualification.
Head SHA: 92311026f6f0261d1b716519dc6f16d720fee168
Base SHA: 0d4d22eab316148e500716f86280457e9e840023
Merge base SHA: 0d4d22eab316148e500716f86280457e9e840023
Patch ID: c0d68130e9f92ee6b235b9327f11f33e6fd84a19
Gator payload: 12
Review mode: initial
Previous reviewed SHA: none
Review budget exhausted: no
Maintainer decision required: no
Checks: DCO, vouch, Helm Lint, and Trivy Changes passed. Branch Checks attempt 2 is running; the earlier macOS failure occurred in Rust setup before the build.
E2E: test:e2e applied. The mirror matches the current head. Following E2E Label Help, run 38438104172 was rerun and confirmed queued; test results are pending.
Next state: gator:watch-pipeline

drew

A closed file descriptor should retain the syscall's EBADF behavior.

johntmyers · 2026-10-14T17:39:47Z

gator-agent

PR Review Status

Thanks @drew, I checked your request to preserve EBADF for closed descriptors. The latest commit maps a confirmed missing descriptor to EBADF, while missing tasks and other inspection errors remain distinct and fail closed. The independent follow-up review found no blocking issues in the author-only delta; regression coverage exercises the lookup, both registry paths, and brokered listen.

Blocking findings: None.
Carried findings: None.

Gator metadata

Validation: Focused sandbox network enforcement fix from a verified maintainer.
Docs: No new published interface or configuration; this delta restores native closed-descriptor error behavior.
Checks: Current-head Branch Checks are running; DCO, PR Issue Acceptance, Helm Lint, and Trivy Changes passed.
E2E: test:e2e remains applied; current-head Branch E2E Checks is queued. The label-help rerun instruction concerned the previous head; the new head already has its own required run.
Head SHA: 3e7bcebdf9533a5142b0afb3edda8a4d26a27bc7
Base SHA: 0d4d22eab316148e500716f86280457e9e840023
Merge base SHA: 0d4d22eab316148e500716f86280457e9e840023
Patch ID: ba6f8717eeb7be0ebe5db6f9fa8d5a90a2d6ef4f
Gator payload: 12
Review mode: follow_up
Previous reviewed SHA: 92311026f6f0261d1b716519dc6f16d720fee168
Review budget exhausted: no
Maintainer decision required: no
Next state: gator:watch-pipeline

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

johntmyers · 2026-10-15T18:29:23Z

gator-agent

Merge Ready

The current head preserves the effective patch from the completed independent review, with no blocking findings or unresolved review threads. Thanks @drew for approving this head; I verified your approval and the current-head required checks.

Action required: A maintainer should merge this PR or decide to close it.

Gator metadata

Validation: Focused sandbox descriptor-inspection enforcement fix from a verified maintainer.
Review: The ledger confirms a rebase-equivalent patch; the completed independent review remains applicable.
Docs: No new published interface or configuration; the reviewed change preserves native closed-descriptor errors while failing closed on inspection errors.
Approval: @drew, verified repository admin, approved the current head.
Checks: Branch Checks, Helm Lint, Trivy Changes, DCO, and PR Issue Acceptance passed on the current head.
E2E: test:e2e applied; current-head Branch E2E Checks and OpenShell / E2E passed. No rerun or mirror refresh is needed.
Head SHA: 724ea2e42c8c7217080acf51ebad18088691ebd4
Base SHA: c3ca45f1dd2848a01309569944e06977e73ff47b
Merge base SHA: c3ca45f1dd2848a01309569944e06977e73ff47b
Patch ID: ba6f8717eeb7be0ebe5db6f9fa8d5a90a2d6ef4f
Gator payload: 12
Review mode: already_reviewed
Previous reviewed SHA: 3e7bcebdf9533a5142b0afb3edda8a4d26a27bc7
Next state: gator:merge-ready

johntmyers · 2026-10-15T18:45:30Z

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: Gator reached merge-ready with no blocking findings, maintainer approval, and passing required checks including E2E.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

Gator metadata

Head SHA: 724ea2e42c8c7217080acf51ebad18088691ebd4
Gator payload: 12

peppywolfym-lab · 2026-10-15T18:55:12Z

    if tid == 0 || fd < 0 {
        return Err(io::Error::new(
            io::ErrorKind::InvalidInput,
            "TID must be nonzero and FD must be nonnegative",


"It would be much more helpful for debugging if the error message included the actual values that triggered it. Could we update the string to print the invalid TID and FD values?"

@peppywolfym-lab feel free to open an issue + pr for this

johntmyers added the test:e2e Requires end-to-end coverage label Oct 13, 2026

johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:approval-needed Gator completed review; maintainer approval needed and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Oct 13, 2026

johntmyers requested a review from drew October 13, 2026 20:52

drew reviewed Oct 14, 2026

View reviewed changes

Comment thread crates/openshell-sandbox/src/network_broker.rs

johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback and removed gator:approval-needed Gator completed review; maintainer approval needed labels Oct 14, 2026

johntmyers requested a review from drew October 14, 2026 18:59

drew approved these changes Oct 14, 2026

View reviewed changes

drew enabled auto-merge October 14, 2026 19:00

johntmyers added gator:merge-ready and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Oct 14, 2026

johntmyers disabled auto-merge October 14, 2026 19:44

johntmyers enabled auto-merge October 14, 2026 19:44

johntmyers added 2 commits October 15, 2026 10:48

fix(sandbox): distinguish descriptor lookup outcomes

5879632

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

fix(sandbox): preserve bad-descriptor errors in native fallback

724ea2e

Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>

johntmyers force-pushed the fix/51-socket-handling/johntmyers branch from 3e7bceb to 724ea2e Compare October 15, 2026 17:49

johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status and removed gator:merge-ready labels Oct 15, 2026

johntmyers added gator:merge-ready and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Oct 15, 2026

johntmyers added this pull request to the merge queue Oct 15, 2026

Merged via the queue into main with commit 01087a4 Oct 15, 2026
123 of 125 checks passed

johntmyers deleted the fix/51-socket-handling/johntmyers branch October 15, 2026 18:42

johntmyers removed the gator:merge-ready label Oct 15, 2026

peppywolfym-lab approved these changes Oct 15, 2026

View reviewed changes

peppywolfym-lab suggested changes Oct 15, 2026

View reviewed changes

peppywolfym-lab approved these changes Oct 15, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(sandbox): distinguish descriptor lookup outcomes - #4494

fix(sandbox): distinguish descriptor lookup outcomes#4494
johntmyers merged 2 commits into
mainfrom
fix/51-socket-handling/johntmyers

johntmyers commented Oct 13, 2026

Uh oh!

github-actions Bot commented Oct 13, 2026

Uh oh!

johntmyers commented Oct 13, 2026

Uh oh!

drew left a comment

Uh oh!

Uh oh!

johntmyers commented Oct 14, 2026

Uh oh!

johntmyers commented Oct 15, 2026

Uh oh!

Uh oh!

johntmyers commented Oct 15, 2026

Uh oh!

peppywolfym-lab Oct 15, 2026

Uh oh!

johntmyers Oct 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

johntmyers commented Oct 13, 2026

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

github-actions Bot commented Oct 13, 2026

Uh oh!

johntmyers commented Oct 13, 2026

PR Review Status

Uh oh!

drew left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johntmyers commented Oct 14, 2026

PR Review Status

Uh oh!

johntmyers commented Oct 15, 2026

Merge Ready

Uh oh!

Uh oh!

johntmyers commented Oct 15, 2026

Monitoring Complete

Uh oh!

peppywolfym-lab Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

johntmyers Oct 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants